contracts/interfaces/IERC7984Rwa.sol (1)

106-122: Consider consolidating duplicate interface definitions.

IERC7984RwaComplianceModule has identical method signatures to IComplianceModuleConfidential defined in contracts/interfaces/IComplianceModuleConfidential.sol. Having two identical interfaces can lead to maintenance burden and potential divergence.

Consider either:

1. Having IERC7984RwaComplianceModule extend IComplianceModuleConfidential
2. Using IComplianceModuleConfidential directly throughout the codebase

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@contracts/interfaces/IERC7984Rwa.sol` around lines 106 - 122, The two
interfaces IERC7984RwaComplianceModule and IComplianceModuleConfidential define
identical signatures (isModule, isCompliantTransfer, postTransfer, onInstall,
onUninstall); consolidate them by replacing the duplicate
IERC7984RwaComplianceModule with either an extension or direct reuse of
IComplianceModuleConfidential: update IERC7984RwaComplianceModule to "extend
IComplianceModuleConfidential" or remove IERC7984RwaComplianceModule and update
all references to use IComplianceModuleConfidential, ensuring method names
isModule, isCompliantTransfer, postTransfer, onInstall, and onUninstall remain
available where referenced.

contracts/token/ERC7984/extensions/ERC7984RwaModularCompliance.sol (1)

91-97: Consider explicit return for unsupported module types.

When moduleType is neither ForceTransfer nor Standard, the function returns an uninitialized installed (implicitly false). While this is safe, an explicit return false or a revert would make the behavior clearer.

♻️ Optional: Make the fallback explicit

     function _isModuleInstalled(
         ComplianceModuleType moduleType,
         address module
     ) internal view virtual returns (bool installed) {
         if (moduleType == ComplianceModuleType.ForceTransfer) return _forceTransferComplianceModules.contains(module);
         if (moduleType == ComplianceModuleType.Standard) return _complianceModules.contains(module);
+        return false;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@contracts/token/ERC7984/extensions/ERC7984RwaModularCompliance.sol` around
lines 91 - 97, The _isModuleInstalled function currently falls through without
an explicit return for unsupported ComplianceModuleType values; update
_isModuleInstalled to explicitly return false (or revert if you prefer stricter
behavior) when moduleType is not ForceTransfer or Standard so the function never
relies on an uninitialized installed value—locate the _isModuleInstalled method
and add a final explicit `return false` (or a revert with a clear message) after
the existing conditionals.

test/token/ERC7984/extensions/ERC7984RwaModularCompliance.test.ts (2)

16-34: Consolidate setup using the existing fixture helper.

Setup is duplicated between fixture and beforeEach; using fixture() directly in beforeEach will reduce drift and keep tests easier to maintain.

♻️ Proposed refactor

-  beforeEach(async function () {
-    const [admin, agent1, agent2, holder, recipient, anyone] = await ethers.getSigners();
-    const token = (
-      await ethers.deployContract('$ERC7984RwaModularComplianceMock', ['name', 'symbol', 'uri', admin])
-    ).connect(anyone) as $ERC7984RwaModularCompliance;
-    await token.connect(admin).addAgent(agent1);
-    const complianceModule = await ethers.deployContract('$ComplianceModuleConfidentialMock');
-
-    Object.assign(this, {
-      token,
-      complianceModule,
-      admin,
-      agent1,
-      agent2,
-      recipient,
-      holder,
-      anyone,
-    });
-  });
+  beforeEach(async function () {
+    Object.assign(this, await fixture());
+  });

Also applies to: 37-55

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/token/ERC7984/extensions/ERC7984RwaModularCompliance.test.ts` around
lines 16 - 34, The test defines a local async fixture function that duplicates
setup already performed in beforeEach; replace the duplicated setup by calling
the shared fixture helper directly in beforeEach and remove the local fixture
function to avoid drift — update references to the returned objects (token,
holder, complianceModule, admin, agent1, agent2, recipient, anyone) so they are
obtained from the existing fixture call (e.g., const { token, holder,
complianceModule, admin, agent1, agent2, recipient, anyone } = await fixture();
or similar), and ensure uses of addAgent and contract connections are performed
inside the central fixture/home fixture helper instead of the local fixture in
ERC7984RwaModularCompliance.test.ts.

---

57-57: Remove async keyword from describe callbacks.

At Lines 57, 69, 112, and 161, describe uses async function. Mocha executes describe callbacks synchronously during suite loading; async callbacks are ignored, and tests registered after an await may not be registered. Move async setup to hooks (before, beforeEach) instead.

Proposed fix

-  describe('support module', async function () {
+  describe('support module', function () {

-  describe('install module', async function () {
+  describe('install module', function () {

-  describe('uninstall module', async function () {
+  describe('uninstall module', function () {

-  describe('check compliance on transfer', async function () {
+  describe('check compliance on transfer', function () {

Also applies to: 69-69, 112-112, 161-161

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/token/ERC7984/extensions/ERC7984RwaModularCompliance.test.ts` at line
57, Remove the async keyword from the describe callbacks (e.g., the
describe('support module', ...) and the other describe blocks flagged) because
Mocha runs describe blocks synchronously; any async/await inside them prevents
proper test registration. Move any asynchronous setup/await logic into before or
beforeEach hooks (create async functions like before(async () => { ... }) or
beforeEach(async () => { ... })) and keep the describe callback plain function()
so all it does is declare nested it/describe calls synchronously; update any
references to setup variables to be assigned in those hooks instead of inside
the describe body.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro